Skip to content

Comments

Use NcSelect in NcRichtext#4247

Merged
raimund-schluessler merged 4 commits intomasterfrom
enh/3743/use-ncselect-in-ncrichtext
Jun 23, 2023
Merged

Use NcSelect in NcRichtext#4247
raimund-schluessler merged 4 commits intomasterfrom
enh/3743/use-ncselect-in-ncrichtext

Conversation

@julien-nc
Copy link
Contributor

@julien-nc julien-nc commented Jun 22, 2023

refs #3743

Switch to NcSelect in NcProviderList and NcSearch from NcRichText.

I couldn't figure out a way to avoid an issue with NcSearch. When selecting a "Load more..." item, even if we actually select the item while searching for more results, the selection jumps to the top and then back to the "more" item.

Not so pretty...but not so bad.

vokoscreenNG-2023-06-22_13-27-31.mp4

@julien-nc julien-nc added 3. to review Waiting for reviews technical debt feature: richtext Related to the richtext component labels Jun 22, 2023
@julien-nc julien-nc added this to the 8.0.0 milestone Jun 22, 2023
@julien-nc julien-nc mentioned this pull request Jun 22, 2023
Closed
5 tasks
@julien-nc julien-nc force-pushed the enh/3743/use-ncselect-in-ncrichtext branch from 42fa122 to 218f2ad Compare June 22, 2023 11:38
@Pytal
Copy link
Contributor

Pytal commented Jun 22, 2023

I couldn't figure out a way to avoid an issue with NcSearch. When selecting a "Load more..." item, even if we actually select the item while searching for more results, the selection jumps to the top and then back to the "more" item.

lmk if it works with nextcloud-libraries/vue-select#20 :)

raimund-schluessler
raimund-schluessler approved these changes Jun 23, 2023
View reviewed changes
Copy link
Contributor

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just the timeout issue makes me wondering.

@julien-nc
Copy link
Contributor Author

julien-nc commented Jun 23, 2023

lmk if it works with nextcloud-libraries/vue-select#20 :)

Interesting. I'll check if it helps.

julien-nc added 2 commits June 23, 2023 11:03
@julien-nc
make sure keys of search result items are unique in NcRichtext's NcSe…
207433f 
…arch

Signed-off-by: Julien Veyssier <[email protected]>
@julien-nc
use future resetFocusOnOptionsChange NcSelect prop to fix flickering …
0748718 
…issue when 'searching for more' in NcRichtext's NcSearch

Signed-off-by: Julien Veyssier <[email protected]>
@julien-nc
Copy link
Contributor Author

julien-nc commented Jun 23, 2023

@Pytal Much better with resetFocusOnOptionsChange 💙 Looking forward to it.
It's used and set to false in NcSearch even if it's not in NcSelect yet. Are we fine with that?

vokoscreenNG-2023-06-23_11-07-41.mp4

@julien-nc
Copy link
Contributor Author

julien-nc commented Jun 23, 2023

Added another commit to make sure the option keys are unique. (It's just setting the id attribute of the option objects, I didn't wanna define the getOptionKey prop).

@Pytal
Copy link
Contributor

Pytal commented Jun 23, 2023

@Pytal Much better with resetFocusOnOptionsChange 💙 Looking forward to it. It's used and set to false in NcSearch even if it's not in NcSelect yet. Are we fine with that?

Great! We can merge the bump #4264 too!

susnux
susnux approved these changes Jun 23, 2023
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@raimund-schluessler raimund-schluessler merged commit 68b3f9e into master Jun 23, 2023
@raimund-schluessler raimund-schluessler deleted the enh/3743/use-ncselect-in-ncrichtext branch June 23, 2023 18:03
@skjnldsv skjnldsv mentioned this pull request Jun 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux approved these changes

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

+1 more reviewer

@Pytal Pytal Pytal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

3. to review Waiting for reviews feature: richtext Related to the richtext component technical debt

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@julien-nc @Pytal @susnux @raimund-schluessler